feat: add text/chunk connector for RAG ingest (#153)#170
Conversation
Completes the RAG ingest pipeline: text/chunk -> ai/embed (batch) -> kb/upsert (batch). - text/chunk splits a document into overlapping chunks by characters (Unicode-aware) or words, with configurable chunk_size/chunk_overlap. output.chunks feeds straight into ai/embed's input and kb/upsert's contents. Pure chunkText core with thorough unit tests (char/word, overlap, exact-fit, unicode, shorter-than-size, error cases). - kb/upsert: a single `metadata` object is now broadcast to every row of a batch, so chunked ingest can share one title/source. (`metadatas` still matches per-row.) - rag-ingest example rewritten to chunk -> embed -> upsert; text/chunk documented in the connector reference; RAG guide updated. rag-ingest passes `mantle validate`. Smarter (separator-aware/token) chunking and kb/query metadata filtering remain follow-ups on #153. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WnKgCRfMvFcZAkoVbr8k79
📝 WalkthroughWalkthroughThis PR adds a new ChangesChunking connector and kb metadata broadcast
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/site/src/content/docs/workflow-reference/connectors.md (1)
334-334: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
kb/upsertparams table doesn't reflect new broadcast semantics.The
metadata/metadatasrow still says "the count must match," but the new broadcast behavior means a singlemetadataobject is applied to every row regardless of row count — onlymetadatas(array) must match one-to-one. This directly contradicts the# broadcast to every chunkexample shown a few lines below (line 429) and inkb.go's updatedkbMetadatadoc comment.📝 Proposed doc fix
-| `metadata` / `metadatas` | object / list | No | Optional JSONB metadata per row. If provided, the count must match. | +| `metadata` | object | No | Optional JSONB metadata broadcast to every row (e.g. shared title/source for a batch). | +| `metadatas` | list | No | Optional per-row JSONB metadata. Count must match `content`/`contents`. |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/site/src/content/docs/workflow-reference/connectors.md` at line 334, Update the `metadata` / `metadatas` row in the `kb/upsert` params table to match the new broadcast behavior: a single `metadata` object should be described as applying to every row, while only `metadatas` (the array form) must match row count one-to-one. Use the nearby `kb/upsert` examples and the `kbMetadata` docs in `kb.go` as the source of truth, and remove the outdated “count must match” wording for the singular `metadata` case.
🧹 Nitpick comments (1)
packages/engine/internal/connector/chunk.go (1)
54-69: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHigh overlap ratios can cause quadratic memory blow-up.
step = size - overlapcan be as small as 1 (overlap = size - 1), which makessliceChunksemit roughlynchunks, each of lengthsize. For a large document this yieldsO(n * size)total output memory — e.g. a 100k-character document withchunk_size=1000, chunk_overlap=999would produce ~100k chunks of ~1000 chars each, well over 100MB. Since this connector feeds directly intoai/embed/kb/upsertbatches, this could also translate into a very large downstream batch.Consider bounding
overlapto a fraction ofsize(e.g.,overlap <= size/2) or documenting/capping the maximum chunk count.♻️ Example validation to cap overlap ratio
if overlap >= size { return nil, fmt.Errorf("chunk_overlap (%d) must be less than chunk_size (%d)", overlap, size) } + if overlap > size/2 { + return nil, fmt.Errorf("chunk_overlap (%d) must not exceed half of chunk_size (%d) to avoid excessive chunk counts", overlap, size) + } step := size - overlap🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/internal/connector/chunk.go` around lines 54 - 69, The sliceChunks helper can generate an excessive number of overlapping chunks when step is very small, causing memory blow-up downstream in ai/embed and kb/upsert. Update the chunking logic in sliceChunks (and the overlap-to-step calculation that feeds it) to cap overlap to a safe fraction of size, such as at most half, or otherwise enforce a maximum chunk count before emitting windows. Keep the behavior centered around sliceChunks so the fix is applied where chunk emission is controlled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/site/src/content/docs/workflow-reference/connectors.md`:
- Line 334: Update the `metadata` / `metadatas` row in the `kb/upsert` params
table to match the new broadcast behavior: a single `metadata` object should be
described as applying to every row, while only `metadatas` (the array form) must
match row count one-to-one. Use the nearby `kb/upsert` examples and the
`kbMetadata` docs in `kb.go` as the source of truth, and remove the outdated
“count must match” wording for the singular `metadata` case.
---
Nitpick comments:
In `@packages/engine/internal/connector/chunk.go`:
- Around line 54-69: The sliceChunks helper can generate an excessive number of
overlapping chunks when step is very small, causing memory blow-up downstream in
ai/embed and kb/upsert. Update the chunking logic in sliceChunks (and the
overlap-to-step calculation that feeds it) to cap overlap to a safe fraction of
size, such as at most half, or otherwise enforce a maximum chunk count before
emitting windows. Keep the behavior centered around sliceChunks so the fix is
applied where chunk emission is controlled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 550dd17c-89ee-48da-b2a9-0956f3a8a17b
📒 Files selected for processing (9)
.changeset/text-chunk-connector.mdpackages/engine/internal/connector/chunk.gopackages/engine/internal/connector/chunk_test.gopackages/engine/internal/connector/connector.gopackages/engine/internal/connector/kb.gopackages/engine/internal/connector/kb_test.gopackages/site/src/content/docs/rag-guide.mdpackages/site/src/content/docs/workflow-reference/connectors.mdpackages/site/src/content/examples/rag-ingest.yaml
Completes the RAG ingest pipeline: text/chunk -> ai/embed (batch) ->
kb/upsert (batch).
metadataobject is now broadcast to every row of a batch, so chunked ingest can share one title/source. (metadatasstill matches per-row.)mantle validate.Smarter (separator-aware/token) chunking and kb/query metadata filtering remain follow-ups on #153.
Claude-Session: https://claude.ai/code/session_01WnKgCRfMvFcZAkoVbr8k79
Summary
What does this PR do?
Changes
Testing
make testpassesmake lintpassesRelated Issues
Closes #
Summary by CodeRabbit
New Features
Documentation
Tests